Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bit integration docker #23

Open
wants to merge 8 commits into
base: initial-bit-integration
Choose a base branch
from

Conversation

epicadk
Copy link

@epicadk epicadk commented Mar 8, 2021

Description

Added docker files to make setup easier.

Type of Change:

  • Code
  • Quality Assurance
  • User Interface
  • Outreach
  • Documentation

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update (software upgrade on readme file)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged
  • Update Postman API at /docs folder
  • Update Swagger documentation and the exported file at /docs folder
  • Update requirements.txt

Code/Quality Assurance Only

  • My changes generate no new warnings
  • My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been published in downstream modules

replace db uri

add Flask-Cors to requirements.txt

linting with black formatter

remove fetch env variables duplication

change pull-push branch for github action to bit branch

add postgresql test db

add shell_context_processor

Add -h postgres to psql command

Place PGPASSWORD at the start of psql command

Add env for running test

* Update main.yml (Patch anitab-org#21)

Co-authored-by: Aditya Kurkure

use non-macOS syntax for db_uri and add db envs

Add postgres db url for github action

Add test for db_test_uri

remove hard coded test db uri

Add psycopg2

Fix db type to postgres+psycopg2

add env to Generate coverage report

move env to global runner

focus pytest cov on tests folder

Remove print command
@epicadk
Copy link
Author

epicadk commented Mar 8, 2021

Oops I accidently added another commit as well. I'll rebase the changes.

@epicadk epicadk force-pushed the bit-integration-docker branch from 53aefc2 to d897e73 Compare March 8, 2021 06:49
@epicadk epicadk marked this pull request as ready for review March 9, 2021 13:37
@mtreacy002 mtreacy002 force-pushed the initial-bit-integration branch from 21a3956 to 163502a Compare March 10, 2021 01:44
migrate = Migrate(app, db)

cors.init_app(app, resources={r"*": {"origins": "http:BIT:5000"}})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use 2 origins? one from docker (this one) and the other one is the localhost:5000 if we run the app locally

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use 2 origins? one from docker (this one) and the other one is the localhost:5000 if we run the app locally

Yup I'll add them

if __name__ == "__main__":
# if we use docker then we can also run it on the default 5000 port
Copy link
Owner

@mtreacy002 mtreacy002 Mar 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep both. So, give options for people who want to run locally as well using port 4000. we can comment that option out when it's not being used.

@@ -3,4 +3,12 @@ COPY ./requirements.txt /dockerBuild/requirements.txt
WORKDIR /dockerBuild
RUN pip install --no-cache-dir -r requirements.txt
COPY . /dockerBuild
ENV DB_TYPE=postgresql
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to use the same docker env for db postgres that being used in BIT backend?
Screen Shot 2021-03-10 at 9 56 02 pm

Bcoz that's how the 2 backends connect to the postgres locally.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is the same host. Let me see where the POSTGRES_HOST and the POSTGRES_PORT and being used and i'll get back to you , but what I can tell you is that they are connected to the same postgre database.

@mtreacy002 mtreacy002 force-pushed the initial-bit-integration branch 3 times, most recently from 3ab6260 to 928bbe5 Compare March 13, 2021 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants